Skip to content

refactor(ci): split audit from prebuild #3298

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
May 6, 2021
Merged

refactor(ci): split audit from prebuild #3298

merged 1 commit into from
May 6, 2021

Conversation

oxy
Copy link

@oxy oxy commented May 6, 2021

Move dependency audits from prebuild to their own jobs, so that an error does not affect the rest of the build/test process.

@oxy oxy requested a review from a team as a code owner May 6, 2021 17:58
Copy link
Contributor

@jsjoeio jsjoeio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor change (I think) but otherwise, this looks way better 🙌

Comment on lines 77 to 80
- name: Install dependencies
if: steps.cache-yarn.outputs.cache-hit != 'true'
run: yarn --frozen-lockfile

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The codecov upload relies on the coverage being generated, which happens when we run the unit tests. So if this is a fresh checkout of the repo, you may need to add a step after install dependencies and before upload code coverage to run the unit tests.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, I'll revert that change if it depends on the unit tests in the first place.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feel free to add a comment too (apologies, I should have done that when I added it!)

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In hindsight it makes perfect sense that you need to run tests to know what coverage for the tests is like - I just didn't notice in a hurry, heh. Pushed before I could add a comment, fingers crossed its fine.

@jsjoeio jsjoeio added the ci Issues related to ci label May 6, 2021
@jsjoeio jsjoeio added this to the v3.9.4 milestone May 6, 2021
Move dependency audits from prebuild to their own jobs,
so that an error does not affect the rest of the build/test process.
@oxy oxy force-pushed the oxy/ci-split branch from 62da7c0 to 5e5280f Compare May 6, 2021 18:06
@oxy oxy changed the title refactor(ci): split codecov/audit from prebuild refactor(ci): split audit from prebuild May 6, 2021
@codecov
Copy link

codecov bot commented May 6, 2021

Codecov Report

Merging #3298 (5e5280f) into main (af5a1c9) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@           Coverage Diff           @@
##             main    #3298   +/-   ##
=======================================
  Coverage   57.54%   57.54%           
=======================================
  Files          24       24           
  Lines        1279     1279           
  Branches      290      290           
=======================================
  Hits          736      736           
  Misses        441      441           
  Partials      102      102           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update af5a1c9...5e5280f. Read the comment docs.

Copy link
Contributor

@jsjoeio jsjoeio left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good work!

@oxy oxy merged commit d27b12b into main May 6, 2021
@oxy oxy deleted the oxy/ci-split branch May 6, 2021 19:02
@jsjoeio jsjoeio added the chore Related to maintenance or clean up label May 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore Related to maintenance or clean up ci Issues related to ci
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants